fix default value of hub_url in config #1139
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Actually now I am thinking that instead of my first changes, it would be better to set the default value in what do you think? |
minrk
reviewed
Aug 26, 2020
Co-authored-by: Min RK <[email protected]>
|
@minrk I am not sure how to continue here. Could you do another review? |
consideRatio
added
code:helm-chart
I agree! That is what we do in z2jh as well I think, and it makes it clearer to the reader of the config than if the default value is different from the configured empty string etc. @bitnik this LGTM, sorry you have had to experience such a delay on getting review help. I'm 100% committed to helping this get merged and could even take over if you want. Thanks for your work in BinderHub!!!!!! ❤️ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In gesiscss/persistent_binderhub#5 we found out that if someone installs BinderHub with auth enabled,
deployment fails with
Error: render error in "persistent_binderhub/charts/binderhub/templates/deployment.yaml": template: persistent_binderhub/charts/binderhub/templates/deployment.yaml:98:74: executing "persistent_binderhub/charts/binderhub/templates/deployment.yaml" at <"/">: invalid value; expected stringThe error indicates
binderhub/helm-chart/binderhub/templates/deployment.yaml
Line 98 in 23ae206
BinderHub.hub_urlis missing during the first round of the installation (https://binderhub.readthedocs.io/en/latest/zero-to-binderhub/setup-binderhub.html#install-binderhub), because it is provided after the first installation when JupyerterHub is ready (https://binderhub.readthedocs.io/en/latest/zero-to-binderhub/setup-binderhub.html#connect-binderhub-and-jupyterhub).This PR fixes the default value of
BinderHub.hub_urlin helm config, so helm won't complain about it during the first installation and then user can setBinderHub.hub_urlwhen it is available. It also fixes itbinderhub_config.py, so it uses default value of hub_url "" instead ofLazyConfigValue.